-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update artifact handling #80
Conversation
…report to the new v4 versions and handle the change to immutable objects
@@ -85,11 +86,22 @@ runs: | |||
sed -e 's|${{ inputs.strip_path }}||' -i ${FILE} | |||
COVERAGE_FILES="${COVERAGE_FILES},${FILE}" | |||
diff -y --color=always "${FILE}.orig" "${FILE}" || true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here file is not removed after checking diff
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This helps with debugging. As we have to consolidate all those coverage-reports into one single artifact, we can have the modified versions alongside the unmodified files in that artifact.
Before, the coverage-reports artifact only contained the unmodified files as it was created by the test jobs before and we only unpacked it here. Now, that we have to create a single artifact and delete all those single files created by the previous jobs, we can as well add originals and patched files together and if someone wonders, why files are not covered, he can see what the pattern replacement did.
with: | ||
name: testplan | ||
name: testplan-000_header |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here testplan name is not same as append_report
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason for this whole change is that we can't have a single "testplan" artifact which is created once and then every job adds stuff to it. As I wrote in the explanation above, these artifacts are now immutable. They can't be changed any more. So we create a lot of small artifacts which all start with testplan- instead. In the generate-report step, we unpack all those testplan-* artifacts, generate our reports, pack everything together in the final "testplan" artifact and delete all the testplan-* artifacts as their content is now in the single testplan artifact.
uses: actions/checkout@v4 | ||
|
||
- name: 'clean_cache' | ||
- name: 'Clean Cache' | ||
if: ${{ steps.generate_report.outputs.overall_status == 'success' }} | ||
uses: 'OXID-eSales/github-actions/clean_cache@v3' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here it is v3
@@ -102,7 +102,7 @@ runs: | |||
|
|||
- name: Upload Log | |||
if: ${{ always() && steps.diff.outputs.skip == 'false' }} | |||
uses: actions/upload-artifact@v3 | |||
uses: actions/upload-artifact@v4 | |||
with: | |||
name: ${{ inputs.logfile_artifact }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this logfile_artifact is removed right
@@ -669,7 +681,7 @@ sonarcloud: | |||
custom_script_container: '' | |||
|
|||
coverage: | |||
prefix: *coverage_prefix | |||
prefix: coverage-report |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isin't coverage-reports-sonarcloud
@@ -622,7 +634,7 @@ runtest: | |||
prefix: 'deprecated_tests_artifacts' | |||
coverage: | |||
path: 'source/deprecated_tests_coverage.xml' | |||
prefix: *coverage_prefix | |||
prefix: coverage-report-runtest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
coverage-reports-runtest
This pull request updates our usage of upload-artifact and download-artifact to v4. They introduced a breaking change which makes artifacts immutable, so we can't add more files to an already created artifact, a feature I relied on quite heavily. These are the changes: